fix(model): exclude system-schema columns from PostgreSQL table introspection - #3356

Wheels Bot — Reviewer

TL;DR — This PR fixes #3349: a PostgreSQL table whose name collides with an ANSI information_schema view (sequences, tables, columns, …) was picking up phantom catalog columns via an unrestricted cfdbinfo(type="columns"), then dying with key [RV] doesn't exist. The fix — a shared $excludeSystemSchemaRows() filter on wheels.Global, wired into both the model adapter and the migrator, plus a default: branch in $getType() — is well-targeted, correctly placed behind the cacheDatabaseSchema memo, and covered by engine-agnostic specs. Verdict: comment. The only blocker-adjacent item is a batch of accidentally-committed test artifacts; the framework change itself is clean.

Conventions

7 unrelated MockBox stub artifacts committed — public/testbox/system/stubs/{139184C0…,16CA3942…,30942F4D…,754BEF48…,B7681C49…,C383511C…,CDCB73D6…}.cfm. These are runtime-generated TestBox mock stubs for SSE / channel helpers (closeSSEStream, sendSSEComment, checkError, sendSSEEvent, initSSEStream, poll, $getChannelEngine) — e.g.:

variables[ "closeSSEStream" ] = variables[ "tmp_closeSSEStream_139184C0543CCDB338AEFB643110CB89" ];

git show cba6a1d21 --stat confirms all 7 land in the same commit as the PostgreSQL fix (12 files changed), and each is a new file. They have nothing to do with #3349 — MockBox regenerates these under content-hashed names on every run, so committing stale copies just pollutes the tree. Please git rm the seven public/testbox/system/stubs/*.cfm additions (and consider a .gitignore entry for public/testbox/system/stubs/ so they don't recur). This is a hygiene nit, not a functional regression — hence the non-blocking verdict.

Tests

Coverage is solid and, deliberately, engine-agnostic — the filter is driven with a hand-built catalog result rather than a live PostgreSQL connection (SystemSchemaColumnFilterSpec.cfc:11), so it runs on every engine × DB leg. It exercises the multi-schema drop, crdb_internal/pg_extension, column-list preservation, the missing-table_schem passthrough, case-insensitivity, and both $getType() branches. The catch block correctly mutates a struct field rather than a local (SystemSchemaColumnFilterSpec.cfc:713-717), which is the BoxLang-safe pattern (Cross-Engine Invariant 11).

One gap worth noting (not blocking): the two integration seams — PostgreSQLModel.$getColumnInfo() and the migrator/Base.cfc call site — are not exercised by an automated spec; the PR body states they're verified against a real PostgreSQL container out-of-band. Given the filter logic itself is unit-covered and the seams are one-line delegations, that's an acceptable split.

Cross-engine

No issues. $excludeSystemSchemaRows() is public with a $ prefix on wheels.Global (Invariant 7), its for (local.i = …) loops are not inside a finally (Invariant 12), and it uses only portable QueryNew/QueryAddRow/QuerySetCell/arguments.columns[col][row] idioms. The override signature matches Base.cfc:559 and databaseAdapters/Base.cfc extends wheels.Global, so the shared helper resolves on both hierarchies.

Docs

Changelog fragment present and correctly named: changelog.d/3349-postgres-system-schema-columns.fixed.md (.fixed.md type suffix, no direct CHANGELOG.md edit). Good.

Commits

fix(model): exclude system-schema columns from PostgreSQL table introspection — valid type/scope, subject well under 100 chars, describes the why. Conforms to commitlint.config.js.

Wheels Bot — Reviewer

TL;DR — This PR fixes #3349: on PostgreSQL/YugabyteDB/CockroachDB an unrestricted cfdbinfo(type="columns") matches the table name across every schema on the connection, so an application table whose name collides with an ANSI information_schema view (sequences, tables, columns, …) silently picked up phantom catalog columns typed "information_schema"."sql_identifier" — which no $getType() case matched, so the model died with the opaque key [RV] doesn't exist. The fix is a shared $excludeSystemSchemaRows() filter on wheels.Global, wired into both the model adapter ($getColumnInfo override, behind the cacheDatabaseSchema memo) and the migrator's direct $dbinfo path, plus a default: branch in $getType() that names the offending type. It is well-targeted, correctly placed, cross-engine clean, and unit-covered on every engine × DB leg. The prior review's only finding — seven stray MockBox stub artifacts — has been dropped from the tree. Verdict: approve.

Prior finding resolved

The earlier COMMENTED review flagged seven accidentally-committed public/testbox/system/stubs/*.cfm MockBox artifacts. The current head (42f6e460) no longer contains them — gh pr view --json files lists exactly the five files that belong to the fix (changelog.d/3349-…, Global.cfc, PostgreSQLModel.cfc, migrator/Base.cfc, SystemSchemaColumnFilterSpec.cfc). Resolved.

Correctness

• The override signature matches the base seam exactly (databaseAdapters/Base.cfc:559-564), and $getColumns() wraps $getColumnInfo() in the cacheDatabaseSchema memo (Base.cfc:434-467), so the filter genuinely runs once per datasource+table per application lifetime as the doc block claims — verified, not inferred.
• The filter is safe for the common (non-colliding) table: JDBC returns only public-schema rows, all of which pass the !ListFindNoCase(schemas, table_schem) test, so nothing is dropped. The preserves every column of the source result spec confirms the QueryNew(columnList) rebuild keeps all metadata columns.
• The $getType() default: branch is strictly better than the prior behavior, not a regression: an unmapped type previously left local.rv unassigned and threw key [RV] doesn't exist; it now throws Wheels.UnknownColumnType naming the type. No previously-mapped type is affected (the switch cases are unchanged).

Cross-engine

No issues. $excludeSystemSchemaRows() is public with a $ prefix on wheels.Global (Invariant 7); both databaseAdapters/Base.cfc:1 and migrator/Base.cfc:1 extends="wheels.Global", so the shared helper resolves on both hierarchies. Its for (local.i = …) loops are not inside a finally (Invariant 12), and arguments.columns["table_schem"][local.i] is a bracket read, not a bracket-notation method call, so it dodges the Adobe parser crash (Invariant 4). Idioms are portable QueryNew/QueryAddRow/QuerySetCell. CockroachDBModel does not override $getColumnInfo, so it inherits the filter — matching the PR body's claim about crdb_internal/pg_extension.

Tests

Solid and deliberately engine-agnostic: the filter is driven with a hand-built catalog result (SystemSchemaColumnFilterSpec.cfc) rather than a live PostgreSQL connection, so it runs on every engine × DB leg. It covers the multi-schema drop, crdb_internal/pg_extension, column-list preservation, the missing-table_schem passthrough, case-insensitivity, and both $getType() branches. The catch block mutates a struct field (thrown.type/thrown.message) rather than a local var, which is the BoxLang-safe pattern (Invariant 11).

One acknowledged, acceptable gap (non-blocking): the two integration seams — PostgreSQLModel.$getColumnInfo() and the migrator/Base.cfc call site — are one-line delegations not exercised by an automated spec; the PR body states they were verified against a real PostgreSQL container out-of-band.

Docs

Changelog fragment present and correctly named — changelog.d/3349-postgres-system-schema-columns.fixed.md (.fixed.md type suffix, no direct CHANGELOG.md [Unreleased] edit). Good.

Commits

fix(model): exclude system-schema columns from PostgreSQL table introspection — valid type/scope, subject well under 100 chars, describes the why. Conforms to commitlint.config.js.

Minor observation (non-blocking)

CockroachDBModel.$getType() is its own override and does not carry the new default: branch, so a genuinely-unmapped CockroachDB type would still surface the opaque key [RV] doesn't exist. This is pre-existing and outside #3349's scope — and the $excludeSystemSchemaRows() filter (inherited via $getColumnInfo) already prevents the actual catalog bleed on CockroachDB — so it does not block. Worth a follow-up if you want the friendlier error there too.